internal/raftengine/etcd/quorum_ack.go (1)

59-71: ⚠️ Potential issue | 🟡 Minor

Minor: failed clock_gettime samples silently get buried by the descending sort.

monoclock.Now().Nanos() returns 0 on ClockGettime failure. That 0 gets stored into peerAcks[peerID], and in recomputeLocked the descending sort sinks it to the bottom — so a single peer whose ack was sampled during a seccomp/sandbox clock failure effectively disappears from the quorum computation (another real ack gets promoted into the boundary slot). The tracker reports a satisfied quorum that is one peer short of what the accounting suggests.

This is fail-open in exactly the scenario the rest of the PR is fail-closed about. It's a narrow window (syscall failure on the leader between the ack and recordAck), but cheap to make consistent: either skip the update when now == 0, or filter zeros out of ackBuf before sorting.

🛡️ Proposed fix (option A: skip store on failed sample)

 func (t *quorumAckTracker) recordAck(peerID uint64, followerQuorum int) {
 	if followerQuorum <= 0 {
 		return
 	}
 	now := monoclock.Now().Nanos()
+	if now == 0 {
+		// clock_gettime failed; do not poison peerAcks with a
+		// sentinel that would be silently buried by the sort.
+		return
+	}
 	t.mu.Lock()

Also applies to: 108-126

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/raftengine/etcd/quorum_ack.go` around lines 59 - 71, The issue is
that monoclock.Now().Nanos() can return 0 on failure and that zero timestamp is
stored into quorumAckTracker.peerAcks, which then gets sorted down in
recomputeLocked and can falsely satisfy quorum; change
quorumAckTracker.recordAck to ignore failed clock samples by skipping the map
update when now == 0 (i.e., do not set t.peerAcks[peerID] when the sampled
timestamp is zero), and also add a defensive filter in recomputeLocked to ignore
zero timestamps when building ackBuf so any existing zeros cannot affect the
descending sort; refer to quorumAckTracker.recordAck,
quorumAckTracker.recomputeLocked, and the peerAcks map when making these
changes.

kv/coordinator.go (1)

285-306: ⚠️ Potential issue | 🟠 Major

Don’t warm the lease from a zero clock sample.

Line 306 and Line 458 both extend the caller-side lease from dispatchStart.Add(...) / now.Add(...). If monoclock.Now() failed and returned monoclock.Zero, Instant.Add still produces a positive counter, so the lease can become briefly valid instead of failing closed on clock-read failure.

Proposed fix

 func (c *Coordinate) refreshLeaseAfterDispatch(resp *CoordinateResponse, err error, dispatchStart monoclock.Instant, expectedGen uint64) {
 	if err != nil {
 		// Only invalidate on errors that actually signal leadership
@@
 	lp, ok := c.engine.(raftengine.LeaseProvider)
 	if !ok {
 		return
 	}
+	if dispatchStart.IsZero() {
+		return
+	}
 	c.lease.extend(dispatchStart.Add(lp.LeaseDuration()), expectedGen)
 }
@@
 	idx, err := c.LinearizableRead(ctx)
 	if err != nil {
 		if isLeadershipLossError(err) {
 			c.lease.invalidate()
 		}
 		return 0, err
 	}
-	c.lease.extend(now.Add(leaseDur), expectedGen)
+	if !now.IsZero() {
+		c.lease.extend(now.Add(leaseDur), expectedGen)
+	}
 	return idx, nil
 }

Also applies to: 428-459

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@kv/coordinator.go` around lines 285 - 306, The code extends the caller-side
lease using a clock sample that may be monoclock.Zero (e.g., in
refreshLeaseAfterDispatch and the corresponding read-path that uses
now.Add(...)), which can erroneously warm a lease when clock reading failed;
update both places (the call site that does dispatchStart.Add(...) in
refreshLeaseAfterDispatch and the other site that does now.Add(...)) to first
detect a zero/failed clock sample (compare the Instant to monoclock.Zero or use
its IsZero helper) and, if zero, avoid extending the lease (preferably
invalidate or skip extend so we fail closed) instead of calling Instant.Add;
keep the existing logic for non-zero Instants.